Skip to content

Fix memory crash/perf. issue with arima.py + exogenous regressors#1015

Open
belliriccardo wants to merge 3 commits intoNixtla:mainfrom
belliriccardo:patch-1
Open

Fix memory crash/perf. issue with arima.py + exogenous regressors#1015
belliriccardo wants to merge 3 commits intoNixtla:mainfrom
belliriccardo:patch-1

Conversation

@belliriccardo
Copy link

@belliriccardo belliriccardo commented May 30, 2025

This simple patch would fix the issue described in #1006; also, by not computing the full matrices in one case and just the singular values in the other, there would also be big performance gains, depending on the time series, regressors, etc.

@belliriccardo
Fix memory crash/performance issue within arima.py when using exogeno…
11c73cb 
…us regressors

Fixes issue Nixtla#1006: Memory crash/performance issue within arima.py when using exogenous regressors
@review-notebook-app
Copy link

review-notebook-app bot commented May 30, 2025

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@CLAassistant
Copy link

CLAassistant commented May 30, 2025 •
edited
Loading

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ belliriccardo
❌ elephaint
You have signed the CLA already but the status is still pending? Let us recheck it.

@elephaint
Copy link
Contributor

elephaint commented Jul 4, 2025

@belliriccardo Thanks for your contribution!

elephaint
elephaint requested changes Jul 4, 2025
View reviewed changes
Copy link
Contributor

@elephaint elephaint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a simple improvement, thanks!

Note that we use nbdev to create the code; hence, please run nbdev_export when making changes to the ipynb files.

@belliriccardo
Copy link
Author

belliriccardo commented Jul 11, 2025

Hey @elephaint ! Sorry I'm late, I've just seen this. Thanks for the feedback; I didn't know about nbdev_export, but I can see that you already merged the changes. Do I still need to run the command or do something else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@elephaint elephaint elephaint requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@belliriccardo @CLAassistant @elephaint